Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add copy command #1044

Merged
merged 1 commit into from
Jan 20, 2025
Merged

Add copy command #1044

merged 1 commit into from
Jan 20, 2025

Conversation

Prajjwal
Copy link
Contributor

Closes #753

@Prajjwal Prajjwal force-pushed the sin.add-copy-command branch from 565421b to 7affd25 Compare December 11, 2024 20:56
HELP

def execute(arg)
output = irb_context.workspace.binding.eval(arg)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we need to apply the output inspection method here so the copied output will be closer to what the users may expect.

For example, the output here by default will be the to_s result of the evaluation result, but IRB's default output inspector pretty prints the result inspect.

Additionally, we need to make sure colorizing is NOT applied to the copied value.

Copy link
Contributor Author

@Prajjwal Prajjwal Dec 16, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem with using the pretty printer is that colorizing output is hardcoded into it, and I don't think we want it in our clipboard.

Unless there's a pretty printer that I've missed, the options are:

  1. Grab colorized output, strip color before copying. Least invasive, and will respect the currently configured inspector.
  2. Install a new pretty printer analogous to the default color printer that does basically the same thing but doesn't colorize output. Don't like this one because we're conflating two concepts and hardcoding a command to use a specific kind of inspection method.
  3. Like above, color_printer.rb to accept a parameter that turns colorizing on / off. Most invasive.

I like 1, but will defer.

@Prajjwal Prajjwal force-pushed the sin.add-copy-command branch from 7affd25 to 8819e5e Compare January 12, 2025 21:00
lib/irb/default_commands.rb Outdated Show resolved Hide resolved
lib/irb/color_printer.rb Outdated Show resolved Hide resolved
lib/irb/command/copy.rb Outdated Show resolved Hide resolved
lib/irb/command/copy.rb Outdated Show resolved Hide resolved
@st0012 st0012 added the enhancement New feature or request label Jan 14, 2025
@Prajjwal Prajjwal force-pushed the sin.add-copy-command branch from 8819e5e to 1b409ba Compare January 14, 2025 17:27
end

def clipboard_program
@clipboard_program = "pbcopy" if executable?("pbcopy")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we also have return if defined?(@clipboard_program) so we don't need to reevaluate the presence of the executables?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I updated it to be @clipboard_program ||=, but it looks like the command is re-instantiated each time so it doesn't matter. I could cache the value elsewhere (say, the class), but that has a subtle bug where changes to IRB.conf[:CLIPBOARD_PROGRAM] won't reflect once this has been computed.

@Prajjwal Prajjwal force-pushed the sin.add-copy-command branch 2 times, most recently from 914962e to 590af2b Compare January 14, 2025 22:34
Comment on lines +174 to +176
ENV['IRB_COPY_COMMAND'] = ''
IRB.setup(__FILE__)
assert_equal('', IRB.conf[:COPY_COMMAND])
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unsure what the behavior here should be. Setting this env var overrides the other clipboard programs we use, so right now this would cause copy to stop working.

However, we could just as easily interpret '' to mean "fall back to pbcopy".

Comment on lines +230 to +231
.It Ev IRB_COPY_COMMAND
Overrides the default program used to interface with the system clipboard.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

screenshot

README.md Outdated
@@ -358,6 +358,7 @@ irb(main):002> a.first. # Completes Integer methods
- `NO_COLOR`: Assigning a value to it disables IRB's colorization.
- `IRB_USE_AUTOCOMPLETE`: Setting it to `false` disables IRB's autocompletion.
- `IRB_COMPLETOR`: Configures IRB's auto-completion behavior, allowing settings for either `regexp` or `type`.
- `IRB_COPY_COMMAND`: Overrides the default program used to interface with the system clipboard.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does the documentation need to be updated anywhere other than here and the man page?

@Prajjwal Prajjwal force-pushed the sin.add-copy-command branch 2 times, most recently from 0f721f1 to f0faf98 Compare January 14, 2025 22:45
@Prajjwal Prajjwal changed the title [DRAFT] - Add copy command Add copy command Jan 15, 2025
end
rescue StandardError => e
warn "Error: #{e}"
warn "Is IRB.conf[:COPY_COMMAND] set to a bad value?"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Currently, copy typo will show this COPY_COMMAND warning message.
I think this message should be placed inside def copy_to_clipboard

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated! Here's how it works now:

λ  irb → λ git sin.add-copy-command → ./bin/console
irb(main):001> copy typo
Error: undefined local variable or method `typo' for main
irb(main):002> copy 5
Copied to system clipboard
irb(main):003> IRB.conf[:COPY_COMMAND] = 'lulz'
=> "lulz"
irb(main):004> copy 5
No such file or directory - lulz
Is IRB.conf[:COPY_COMMAND] set to a bad value?
irb(main):005>

@Prajjwal Prajjwal force-pushed the sin.add-copy-command branch from f0faf98 to f308c9e Compare January 17, 2025 04:40
@Prajjwal Prajjwal force-pushed the sin.add-copy-command branch from f308c9e to b4eb6be Compare January 17, 2025 04:52
Copy link
Member

@st0012 st0012 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is great, thank you ❤️

@st0012 st0012 merged commit a24ac53 into ruby:master Jan 20, 2025
34 checks passed
@st0012 st0012 deleted the sin.add-copy-command branch January 20, 2025 21:40
tompng pushed a commit to tompng/ruby that referenced this pull request Jan 22, 2025
tompng pushed a commit to ruby/ruby that referenced this pull request Jan 22, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Development

Successfully merging this pull request may close these issues.

Explore the possibility to add a command to copy evaluation result
3 participants